[PM-1214] device management screen #12455

alec-livefront · 2024-12-17T22:27:54Z

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-1214?atlOrigin=eyJpIjoiOGUyMGIwODJmODQwNDlkMjlhZmFhZDYxNmVjN2EyZGEiLCJwIjoiaiJ9

📔 Objective

Adds a device management tab under settings -> security. Allows a user to manage the devices of their account by approving them, logging them out or deleting them.

📸 Screenshots

GMT20241220-201106_Clip_Alec.Rippberger.s.Clip.12_20_2024.mp4

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

github-actions · 2024-12-17T22:36:16Z

Checkmarx One – Scan Summary & Details – f0618a5c-1e6e-4d2c-a85b-dac113542afc

No New Or Fixed Issues Found

codecov · 2024-12-17T22:38:13Z

Codecov Report

Attention: Patch coverage is 5.33333% with 71 lines in your changes missing coverage. Please review.

Project coverage is 33.73%. Comparing base (aeba2b3) to head (78ac02d).

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...h/settings/security/device-management.component.ts	0.00%	60 Missing ⚠️
...services/devices/devices.service.implementation.ts	0.00%	6 Missing ⚠️
...c/app/auth/settings/security/security.component.ts	33.33%	1 Missing and 1 partial ⚠️
apps/browser/src/background/main.background.ts	0.00%	1 Missing ⚠️
.../auth/settings/security/security-routing.module.ts	0.00%	1 Missing ⚠️
...uth/services/devices-api.service.implementation.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #12455   +/-   ##
=======================================
  Coverage   33.73%   33.73%           
=======================================
  Files        2918     2919    +1     
  Lines       91001    91071   +70     
  Branches    17201    17217   +16     
=======================================
+ Hits        30696    30724   +28     
- Misses      57905    57947   +42     
  Partials     2400     2400

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

alec-livefront · 2024-12-18T04:13:22Z

I could use help on a few things I'm unsure of on this PR:

The ticket calls for virtual scroll on the table. However, when I try to implement bit-table-scroll I get Unhandled error in angular TypeError: Cannot read properties of undefined (reading 'template'). I looked at the vnext-clients component to see if I could spot what might be different here but came up empty.
I'm not sure how to implement the removeDevice, ~~approveDevice or logOutDevice~~ functions (or if it is currently possible with the backend). I tried to flush out removeDevice using the devices/{id}/deactivate endpoint but I don't believe it is working. I could use some direction on what to do here.
The Figma designs seem to show relative times for the last login (like "This week", "Last month". I'm wondering if there is existing logic to convert a date to a human readable time exists elsewhere in the app? I couldn't find anything. It shouldn't be hard to write but I wanted to be consistent. Another issue is I'm wondering how to get the last login date - it doesn't seem to be included in the device info from the backend.

…b.com/bitwarden/clients into auth/pm-1214/device-management-screen

libs/common/src/auth/services/device-trust.service.implementation.ts

JaredSnider-Bitwarden

This is looking great! I've got a few UI suggestions and some minor clean up items below:

apps/web/src/app/auth/settings/security/device-management.component.html

apps/web/src/app/auth/settings/security/device-management.component.ts

JaredSnider-Bitwarden · 2024-12-20T19:39:58Z

apps/web/src/app/auth/settings/security/security-routing.module.ts

+      {
+        path: "device-management",
+        component: DeviceManagementComponent,
+        data: { titleId: "devices" },
+      },


⚠️ : I believe this should be gated behind the feature flag guard for web app approvals, right?

@JaredSnider-Bitwarden Per this convo w/ Todd I think we're going to test this on the feature branch? https://bitwarden.atlassian.net/browse/PM-1214?focusedCommentId=80794 I've removed the feature flag here: 3ed12c6

Ah, ok. I'm sorry for leading you in the wrong direction with this comment. I forgot that was decided.

libs/common/src/auth/abstractions/devices/devices.service.abstraction.ts

apps/web/src/app/auth/settings/security/device-management.component.ts

libs/common/src/auth/services/device-trust.service.spec.ts

libs/common/src/auth/services/devices-api.service.implementation.spec.ts

…known" platform correctly

apps/web/src/app/auth/settings/security/device-management.component.html

rr-bw · 2024-12-27T22:31:42Z

apps/web/src/app/auth/settings/security/device-management.component.html

+    <i
+      class="bwi bwi-question-circle tw-flex tw-items-center tw-h-4 tw-w-4 bwi-color-primary"
+      aria-hidden="true"
+    ></i>


Is there a corresponding link/popover text for this question mark icon?

I have an open question re. that on the FIgma: https://www.figma.com/design/girHiInJR1CRjLYvXQDIEl?node-id=3411-81480#1076109108

@alec-livefront , everything is looking solid here! Just waiting on this last item before approval!

apps/web/src/app/auth/settings/security/device-management.component.html

alec-livefront added 11 commits December 6, 2024 21:33

Add basic device management screen

79327d8

Add table styling

d4c0091

Add options button

efda9ae

Merge branch 'main' into auth/pm-1214/device-management-screen

a4d1740

Fix bad merge

dc2596e

Merge branch 'main' into auth/pm-1214/device-management-screen

4c49954

Fix icons, trusted status, login status and last login

5eb9955

Add sorting and table description.

99f6af0

Add table button functionality

f1a59e9

Add device deactivation

ebddea0

Add strict typing and other cleanup

b078c74

alec-livefront added 3 commits December 17, 2024 16:41

Type fixes and message update

660aba2

Update login status column with pending auth request

80760c8

Update table styling

278fa9b

alec-livefront changed the title ~~Auth/pm 1214/device management screen~~ [PM-1214] device management screen Dec 18, 2024

Merge branch 'main' into auth/pm-1214/device-management-screen

eef346a

alec-livefront requested a review from JaredSnider-Bitwarden December 18, 2024 04:13

alec-livefront and others added 9 commits December 18, 2024 08:39

Merge branch 'main' into auth/pm-1214/device-management-screen

74363fb

Add virtual scroll to table

bcd847d

Merge branch 'main' into auth/pm-1214/device-management-screen

0561cd8

Remove approve device and log out menu options

f336afc

Merge branch 'auth/pm-1214/device-management-screen' of https://githu…

82dbc98

…b.com/bitwarden/clients into auth/pm-1214/device-management-screen

Remove device when successfully deactivated

d995fe5

Update row size

3eb0ef7

Add service tests

c3c73cc

Merge branch 'main' into auth/pm-1214/device-management-screen

6ca912a

alec-livefront marked this pull request as ready for review December 20, 2024 16:09

alec-livefront requested a review from a team as a code owner December 20, 2024 16:09

JaredSnider-Bitwarden reviewed Dec 20, 2024

View reviewed changes

libs/common/src/auth/services/device-trust.service.implementation.ts Outdated Show resolved Hide resolved

JaredSnider-Bitwarden requested changes Dec 20, 2024

View reviewed changes

rr-bw self-requested a review December 20, 2024 19:56

alec-livefront and others added 8 commits December 20, 2024 14:04

Move getCurrentDevice$ to DevicesService

99a647c

Update "Remove" to "Remove device"

d01ca4d

Add loading spinners

0932e18

Use ValidationService to handle error

3934101

Fix platform translation in device management component to handle "Un…

4bb0459

…known" platform correctly

Merge branch 'main' into auth/pm-1214/device-management-screen

c8dc459

Add DeviceManagement feature flag

66e497c

Merge branch 'main' into auth/pm-1214/device-management-screen

59cae5b

alec-livefront requested a review from JaredSnider-Bitwarden December 27, 2024 20:28

rr-bw requested changes Dec 27, 2024

View reviewed changes

JaredSnider-Bitwarden and others added 4 commits December 30, 2024 10:57

Merge branch 'main' into auth/pm-1214/device-management-screen

6e4a873

Clean up device management header to look more like other tabs

3bebfab

Fix default column sort

fa51b93

Merge branch 'main' into auth/pm-1214/device-management-screen

f9f048a

alec-livefront requested a review from rr-bw December 30, 2024 16:54

JaredSnider-Bitwarden removed the request for review from rr-bw December 30, 2024 20:18

alec-livefront and others added 6 commits December 31, 2024 10:12

Merge branch 'main' into auth/pm-1214/device-management-screen

287e9e6

Merge branch 'main' into auth/pm-1214/device-management-screen

8c79141

Merge branch 'main' into auth/pm-1214/device-management-screen

ea2a96d

Remove DeviceManagement feature flag

3ed12c6

Add popover content

f017b27

Merge branch 'main' into auth/pm-1214/device-management-screen

78ac02d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PM-1214] device management screen #12455

[PM-1214] device management screen #12455

alec-livefront commented Dec 17, 2024 •

edited

Loading

github-actions bot commented Dec 17, 2024 •

edited

Loading

codecov bot commented Dec 17, 2024 •

edited

Loading

alec-livefront commented Dec 18, 2024 •

edited

Loading

JaredSnider-Bitwarden left a comment •

edited

Loading

JaredSnider-Bitwarden Dec 20, 2024

alec-livefront Dec 27, 2024

alec-livefront Jan 2, 2025

JaredSnider-Bitwarden Jan 2, 2025

rr-bw Dec 27, 2024

alec-livefront Dec 30, 2024

JaredSnider-Bitwarden Dec 30, 2024

[PM-1214] device management screen #12455

Are you sure you want to change the base?

[PM-1214] device management screen #12455

Conversation

alec-livefront commented Dec 17, 2024 • edited Loading

🎟️ Tracking

📔 Objective

📸 Screenshots

⏰ Reminders before review

🦮 Reviewer guidelines

github-actions bot commented Dec 17, 2024 • edited Loading

No New Or Fixed Issues Found

codecov bot commented Dec 17, 2024 • edited Loading

Codecov Report

alec-livefront commented Dec 18, 2024 • edited Loading

JaredSnider-Bitwarden left a comment • edited Loading

Choose a reason for hiding this comment

JaredSnider-Bitwarden Dec 20, 2024

Choose a reason for hiding this comment

alec-livefront Dec 27, 2024

Choose a reason for hiding this comment

alec-livefront Jan 2, 2025

Choose a reason for hiding this comment

JaredSnider-Bitwarden Jan 2, 2025

Choose a reason for hiding this comment

rr-bw Dec 27, 2024

Choose a reason for hiding this comment

alec-livefront Dec 30, 2024

Choose a reason for hiding this comment

JaredSnider-Bitwarden Dec 30, 2024

Choose a reason for hiding this comment

alec-livefront commented Dec 17, 2024 •

edited

Loading

github-actions bot commented Dec 17, 2024 •

edited

Loading

codecov bot commented Dec 17, 2024 •

edited

Loading

alec-livefront commented Dec 18, 2024 •

edited

Loading

JaredSnider-Bitwarden left a comment •

edited

Loading